Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Configure different mass thresholds for identical & similar issues #47

Closed
wants to merge 5 commits into from

Conversation

wfleming
Copy link
Contributor

One of the things I've noticed recently when running the duplication engine both against real code & just for testing is that generally when it emits issues I think are silly, it's because the the locations are structurally similar, but not identical. Examples of kind of silly issues this results in:

  • complaining that you care calling an attributes macro with two completely different sets of attribute names on two separate classes (i.e. attribute :foo, :bar, :baz is a duplicate of attribute :fleek, :fluck, :fark)
  • complaining that you have an if statement that calls different methods on the same objects in the branches (if (this.someProp) { a.doThing(); b.doAlso(); } else { a.doOtherThing(); b.doOtherAlso(); })
  • complaining that you are catching different exceptions & generating different error messages from them (see recent pr improve derivation of lines from Sexp #44)

I'm of the opinion that generally speaking the bar for similar code being a violation should be higher than identical code. Particularly at relatively low masses (which our defaults all are). Complaining about things like calling methods with different arguments in different places is not just low-value, but negative value IMHO: it's noise that distracts from legitimate issues & makes the engine as a whole seem less reliable.

This won't completely eliminate these instances, but it will make them rarer & easier to filter out without throwing out the baby with the bathwater (the baby in this case being more severe duplication issues that are either identical blocks or bigger similar blocks). I'm also starting to think setting per-file/global levels would also be helpful: i.e. you might want a lower threshold for similar code in the same file than you do for similar code across an entire project.

Thoughts, @codeclimate/review ?

@wfleming wfleming force-pushed the will/threshold-config branch from 7ee54d8 to e2c81cb Compare November 18, 2015 21:21
To adjust this setting, add a `mass_threshold` key with your preferred value for
You can set thresholds for the two different types of duplication this engine reports: blocks that are identical to each other, and blocks that are structurally similar but differ in content.

To adjust these thresholds, you can add `identical_mass_threshold` and `similar_mass_threshold` keys with your preferred value for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you re-flow these paragraphs a bit better? I think the rest of this README (and ours in general) do hard-breaks at 80 columns.

In (I think default) vim,gipq will reflow the paragraph around the cursor with at your given textwidth, which I have at 80.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks.

@wfleming wfleming force-pushed the will/threshold-config branch from e2c81cb to 8c8a990 Compare November 18, 2015 21:28
@wfleming wfleming force-pushed the will/threshold-config branch from 8c8a990 to f17dc4a Compare November 18, 2015 21:29
@pbrisbin
Copy link
Contributor

We should probably get @brynary 's thoughts since I think this boils down to a Product decision.

The code LGTM

@wfleming wfleming closed this Dec 16, 2015
@dblandin dblandin deleted the will/threshold-config branch October 11, 2016 22:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants